Skip to content

fix(server): reclaim expired callback rate-limit windows - #77

Merged
l1shen merged 1 commit into
oomol-lab:mainfrom
l1shen:fix/71-callback-window-cleanup
Sep 5, 2026
Merged

fix(server): reclaim expired callback rate-limit windows#77
l1shen merged 1 commit into
oomol-lab:mainfrom
l1shen:fix/71-callback-window-cleanup

Conversation

@l1shen

@l1shen l1shen commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Expired callback rate-limit windows remained in memory after their endpoints stopped receiving requests. Callback admission now removes expired entries at most once per minute while preserving active windows and their counts. Cleanup is request-driven: idle instances retain entries until the next admission after cleanup is due.

Webhook and Integration routes already resolve endpoints before recording rate-limit state, so arbitrary nonexistent endpoint IDs do not populate the map. This fixes retention of historical valid endpoint keys.

The regression test failed before the fix and now verifies cross-endpoint cleanup, preserved 429/Retry-After behavior for active windows, and admission after expiry.

Validation: bun run format, bun run check, bun run test (1,088 tests), and bun run build passed.

Fixes #71.

@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Essentials

Run ID: 498f8c24-0aef-428c-8b71-5ef93c742e85

📥 Commits

Reviewing files that changed from the base of the PR and between 50fc5f0 and 2fac4f7.

📒 Files selected for processing (2)
  • apps/server/node/http.ts
  • apps/server/test/webhook.test.ts

Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour.


Summary by CodeRabbit

  • Bug Fixes
    • Improved callback rate limiting by automatically clearing expired rate-limit windows.
    • Preserved active limits while ensuring expired callback windows no longer affect new requests.
    • Maintained expected webhook method and status responses.

Walkthrough

The callback rate limiter now tracks the next cleanup time and removes expired callback windows at most once per minute before admission checks. Existing retry behavior remains unchanged. Regression tests mock time and verify that expired webhook windows are removed, active callback limits remain enforced, wait-action windows are preserved, and response statuses and retry-after values remain correct.

Merge Risk: ⚪ Minimal · up to 2fac4

Callback rate-limit entries now expire during later admissions while active limits and retry behavior remain intact. The change is ready to merge.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title uses the required format, "fix(server): ", and accurately describes the callback rate-limit cleanup change.
Description check ✅ Passed The description clearly explains the expired-window cleanup, endpoint behavior, regression coverage, validation, and linked issue.
Linked Issues check ✅ Passed The changes address issue #71 by evicting expired callback rate-limit windows during admission and preserving the existing endpoint-resolution behavior that prevents invalid endpoint keys from creatin…
Out of Scope Changes check ✅ Passed The reported changes are limited to callback rate-limit cleanup and its regression test. No unrelated changes are identified.
✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code

Comment @coderabbitai help to get the list of available commands.

@l1shen
l1shen merged commit 5fe4a4d into oomol-lab:main Sep 5, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Public callback rate limiter stores attacker-controlled keys without eviction

1 participant